Skip to content

Fix media cache deadlocks when switching scenes - #1785

Merged
summeroff merged 3 commits into
stagingfrom
fix-memory-manager-data-races
Sep 29, 2026
Merged

summeroff merged 3 commits into
stagingfrom
fix-memory-manager-data-races

Conversation

@aleksandr-voitenko

Copy link
Copy Markdown
Collaborator

Description

Old memory manager has been rewritten from the ground. Fixes media-cache deadlocks that can freeze scene switching and block source cleanup.

  • Replace per-update threads with a single MediaCacheManager worker that coalesces requests, manages cache reservations, and schedules bounded readiness retries.
  • Run media queries and cache-setting updates through a bounded graphics tick callback, serializing access with source updates that can replace the media player.
  • Track sources by identity, retain references through pending work, and discard stale jobs after updates or removal.
  • Keep OBS calls and source releases outside the queue mutex. Cancel pending work and stop the manager before OBS shutdown, including when video is stopped or no sources remain.

Motivation and Context

Support issue reports scene-switch freezes with animated overlays on Windows, Streamlabs Desktop 1.21.9.

A local reproduction using the supplied WebM files exposed a circular wait between media-cache workers holding different source locks. The graphics thread subsequently blocked during source activation, freezing video processing and preventing cleanup.

This change removes those competing worker locks and makes cache scheduling and source lifetime explicit. Product changes are confined to OSN and use the existing OBS media-source caching setting.

How Has This Been Tested?

Reproduced in a stand-alone test.
Tested via controlled time advances in unit tests + Manually. Windows only.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Tweak (non-breaking change to improve existing functionality)
  • Code cleanup (non-breaking change which makes code smaller or more readable)

Old memory manager has been rewritten from the ground.
Replace per-update threads with a single worker that coalesces requests
and manages cache budgets. Run media queries and cache-setting updates
on the OBS graphics thread, and release source references outside locks.

Cancel pending work safely during removal and shutdown. Add native tests
for budgeting, readiness retries, source lifetime, and video resets.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Advanced Settings updates do not synchronize the preference value read by the cache manager.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread obs-studio-server/source/nodeobs_settings.cpp Outdated
Update the runtime preference when saving Advanced Settings and loading
configuration at startup. Share the update path with SetMediaFileCaching.

Add regression tests for preference changes and startup persistence.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The extensive concurrency and OBS lifecycle rewrite warrants final human validation despite strong regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@summeroff
summeroff merged commit 2702b47 into staging Sep 29, 2026
17 checks passed
// Process only the batch taken above. After a SetCaching job, any follow-up
// QuerySource job must wait for a later tick so ffmpeg_source can process
// its deferred settings update first.
for (auto &result : results) {

@sandboxcoder sandboxcoder Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude-opus comment] potential Bug

memory-manager.cpp:252-259, triggered from osn-source.cpp:357-358

osn::Source::Update calls obs_source_update(src, sets) and then requestCacheUpdate(src). For ffmpeg_source the settings are written straight away, but the plugin's update() (which swaps the media player) is deferred until the source's next video_tick. In libobs, tick callbacks run before the sources are ticked (obs-video.c:48-56, then :78-84).

So when the user changes a looping local source from file A to file B, the first graphics tick after the request runs QuerySource like this:

  • readSettings sees local_file = B (the new settings).
  • get_file_info / get_pla A, because the deferredupdate hasn't run yet.

complete() then reserves A's size and queues an enable job for file == B. That
job passes its check, becay B. After that the wrongreservation never gets corrected:

  • Once caching is on, quens early and neverre-measures.
  • entry.file already equalse.

If B is much bigger than d for as long as the source lives. It can also go the other way and block other sources.

The comment at :249-251 already handles this for the manager's own SetCaching update, but not for updatx: keep a tick counter andonly run a QuerySource on a tick strictly after the one where its request
arrived, which guarantees applied first.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is real. I'll fix it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add that chance to encounter it in the real life is low.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants